-
Notifications
You must be signed in to change notification settings - Fork 161
Improve Admin CLI with JLine #25723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Admin CLI with JLine #25723
Conversation
Thihup
commented
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, @Thihup ! Thanks!
I have a few requests. Especially about including FileNameCompleter and moving the JLine artifact to web profile feature set so that it's also available in Web Profile distribution.
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
|
@OndroMih Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to approve, thanks!
nucleus/admin/cli/src/main/java/com/sun/enterprise/admin/cli/MultimodeCommand.java
Outdated
Show resolved
Hide resolved
|
Problem with |
EDIT: I am wrong. If I am not wrong, that is usual problem with Windows CMD32, can you try with PowerShell? I noticed some time ago that CMD doesn't support |
No. We should pause the asadmin jline terminal before creating the osgi jline terminal and resume it after osgi jline session done. Without this patch osgi shell works fine in the multimode with autocompletion. |
|
Checkstyle complains about using wildcards in imports. |
|
@avpinchuk Could you try again, please? |
|
@Thihup Everything works but auto-completion tips now include the contents of the current directory:
The situation is the same in Linux. |
|
@avpinchuk Great to hear! Showing the file names was a suggestion of @OndroMih to help in commands like |
|
@OndroMih Should we suggest file names for all commands or only for those that take a file name as an argument? |
|
I didn't realize that the FileNameCompleter wouldn't know the context and would offer files for each completion. If it's not too complicated, @Thihup , it would be good to do what @avpinchuk suggested - offer file completion only for commands that take file arguments. If it's too difficult, we can maybe autocomplete only if the pattern is non-empty - e.g. we would complete files for Linux shell now completes files only if the command supports file arguments, or when the last word starts with |
…to feature/improve-admin-cli
|
Now the FileNameCompleter is available only in specific options and when the user specifies a token similar to a path (like |
|
Awesome, @Thihup , you're a coding god! Everything works perfectly when I tested it. Just one thing - when I press Ctrl+D to quit the session, I get this exception before asadmin exits: it happens every time, even if I just start asadmin and press Ctrl+D immediately. When I press Ctrl+C instead, I don't get the exception. |
|
Fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Thank you!
